Skip to content

Conversation

ChrisJBurns
Copy link
Collaborator

  • Rename port to proxyPort - represents proxy runner port
  • Rename targetPort to mcpPort - represents MCP server port

This change addresses user confusion about port semantics as discussed in issue #1452. The new names clearly indicate which port corresponds to which component:

  • proxyPort: Port exposed by the proxy runner in Kubernetes
  • mcpPort: Port that the MCP server listens on internally

Updated all relevant files:

  • CRD types definition and generated YAML
  • Operator controller logic
  • All example and test YAML files
  • Keycloak deployment example

🤖 Generated with Claude Code

claude bot and others added 2 commits August 27, 2025 10:27
- Rename `port` to `proxyPort` - represents proxy runner port
- Rename `targetPort` to `mcpPort` - represents MCP server port

This change addresses user confusion about port semantics as discussed
in issue #1452. The new names clearly indicate which port corresponds
to which component:
- proxyPort: Port exposed by the proxy runner in Kubernetes
- mcpPort: Port that the MCP server listens on internally

Updated all relevant files:
- CRD types definition and generated YAML
- Operator controller logic
- All example and test YAML files
- Keycloak deployment example

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-authored-by: Chris Burns <[email protected]>
jhrozek
jhrozek previously approved these changes Sep 9, 2025
@jhrozek
Copy link
Contributor

jhrozek commented Sep 9, 2025

lol, I approved before looking at the CI run 🤦🏻

@ChrisJBurns
Copy link
Collaborator Author

I'll pull it locally and run the docs steps

@ChrisJBurns ChrisJBurns changed the title Rename MCPServer CRD port attributes for clarity Rename MCPServer CRD Port Attributes for Clarity Sep 18, 2025
Copy link

codecov bot commented Sep 18, 2025

Codecov Report

❌ Patch coverage is 47.05882% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.29%. Comparing base (3a3ff50) to head (7badebd).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...d/thv-operator/controllers/mcpserver_controller.go 35.71% 6 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1806      +/-   ##
==========================================
- Coverage   47.29%   47.29%   -0.01%     
==========================================
  Files         223      223              
  Lines       27681    27681              
==========================================
- Hits        13092    13091       -1     
- Misses      13591    13593       +2     
+ Partials      998      997       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jhrozek
Copy link
Contributor

jhrozek commented Oct 8, 2025

shouldn't we resurrect this PR?

@ChrisJBurns
Copy link
Collaborator Author

@jhrozek ahh yep, let me add this to the list of things to get soon ASAP. I think the only thing thats left is to just make sure we can leave the old fields in for a few versions and make them as deprecated. this is so we don't break existing setups

@ChrisJBurns
Copy link
Collaborator Author

@claude rebase and fix the merge conflicts

Copy link
Contributor

claude bot commented Oct 16, 2025

Claude finished @ChrisJBurns's task —— View job


Rebasing and Fixing Merge Conflicts

I've investigated the current state of the branch and identified the issue, but I'm unable to complete the rebase due to tool permissions.

Current Situation

  • The branch claude/issue-1452-20250827-1021 is behind origin/main by 1 commit
  • That commit is: 0091210 Refactor OIDC resolver to use generic OIDCConfigurable interface (#2228)
  • The branch has 46 commits ahead of main
  • This commit restructured OIDC configuration handling and created new files

What Needs to Happen

The branch needs to be rebased onto the latest origin/main to incorporate that OIDC refactoring commit. Based on my analysis:

  1. No direct file conflicts expected: The OIDC refactor creates new files and modifies files that our PR doesn't touch
  2. However, there's a potential indirect conflict: The OIDC refactor changes the GetPort() method name in the interface, and our PR renames the port field from port to proxyPort

Required Commands

Since I don't have permission to run git rebase or git merge commands, you'll need to run these locally:

# Fetch latest changes
git fetch origin

# Rebase onto main
git rebase origin/main

# If conflicts occur, resolve them and continue
git rebase --continue

# Force push the rebased branch (since rebase rewrites history)
git push origin claude/issue-1452-20250827-1021 --force-with-lease

Potential Conflict to Watch For

The OIDC refactor (commit 0091210) introduced an OIDCConfigurable interface with a GetPort() method. Our PR renames the Port field to ProxyPort in the MCPServer CRD. You may need to:

  1. Update any references to GetPort() to use the new proxyPort field name
  2. Ensure the interface method aligns with the new field naming

Alternative: If you can grant me permission to run git rebase and git merge commands (add them to allowedTools), I can complete the rebase automatically.


Job Run | Branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants